fix: build examples in CI#347
Conversation
Greptile SummaryThis PR adds example compilation to the CI packaging path. The main changes are:
Confidence Score: 5/5The changed flow looks mergeable after tightening build-failure handling and correcting the leak description.
BuildExampleProjectsModule.cs and DisposalExample.cs Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[CI pipeline] --> B[Build examples]
A --> C[Run tests]
A --> D[Generate version]
A --> E[Remove package files]
B --> F[Pack projects]
C --> F
D --> F
E --> F
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart LR
A[CI pipeline] --> B[Build examples]
A --> C[Run tests]
A --> D[Generate version]
A --> E[Remove package files]
B --> F[Pack projects]
C --> F
D --> F
E --> F
Reviews (1): Last reviewed commit: "fix: keep examples compiling in CI" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| { | ||
| results.Add(await context.DotNet().Build(new DotNetBuildOptions | ||
| { | ||
| ProjectSolution = exampleProjectFile.Path, | ||
| Configuration = "Release", | ||
| }, cancellationToken: cancellationToken)); |
There was a problem hiding this comment.
If DotNet().Build returns a nonzero CommandResult rather than throwing, this module stores that result and completes successfully. PackProjectsModule only depends on module completion and never checks these results, so packaging can continue after an example fails to compile.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Fixed in 563971e. BuildExampleProjectsModule now passes CommandExecutionOptions { ThrowOnNonZeroExitCode = true }, so any failed example build throws and fails the module before packaging can run.
| // 🔥 RESOURCE LEAK: The processor goes out of scope without being disposed, | ||
| // potentially leaving tasks running and resources uncleaned | ||
| return results; |
There was a problem hiding this comment.
The new eager await foreach waits for every result before this method returns, so processor tasks are no longer left running when it goes out of scope. The processor still is not disposed, but this example now demonstrates leaked disposal resources rather than unfinished work.
| // 🔥 RESOURCE LEAK: The processor goes out of scope without being disposed, | |
| // potentially leaving tasks running and resources uncleaned | |
| return results; | |
| // 🔥 RESOURCE LEAK: The processor goes out of scope without being disposed, | |
| // leaving its disposal resources uncleaned | |
| return results; |
There was a problem hiding this comment.
Fixed in 563971e. The comment now states that undisposed processor resources remain uncleaned; it no longer claims eager-enumerated tasks are still running.
1 participant
Summary
IAsyncEnumerable.ToList()and inaccessibleToAsyncEnumerable()calls with materializedIReadOnlyList<int>results in disposal examples.BuildExampleProjectsModuleto the ModularPipelines CI graph.Root cause
The example project was present in the solution but absent from the CI module graph. Disposal examples also depended on conversions that are not public in the v4 API.
Test plan
dotnet build EnumerableAsyncProcessor.Example/EnumerableAsyncProcessor.Example.csproj -c Releasedotnet build EnumerableAsyncProcessor.Pipeline/EnumerableAsyncProcessor.Pipeline.csproj -c Releasedotnet build TomLonghurst.EnumerableAsyncProcessor.sln -c Releasedotnet test TomLonghurst.EnumerableAsyncProcessor.sln -c Release— 1,073/1,074 passed; unrelated net8.0 process-wide allocation assertion exceeded its nondeterministic threshold by 5,144 bytes. Follow-up: Fix flaky process-wide allocation assertion #346. net9.0 passed.Closes #341